feat: add an alwaysInline builtin#2895
Conversation
|
@JairusSW let me know if anything else is desired for this builtin |
HerrCai0907
left a comment
HerrCai0907
left a comment
There was a problem hiding this comment.
what is the difference between inline and this builtin?
|
@HerrCai0907 this gives you fine-grained control over when a function is inlined (by specifying it in the relevant call sites, instead of in the definition). @JairusSW has an example as to how this would be useful in as-json. |
|
@CountBleck would it be possible to just rename it to |
I originally intended to name it |
HerrCai0907
left a comment
HerrCai0907
left a comment
There was a problem hiding this comment.
LGTM for implement part.
But I have some concern about design. I think we should avoid to pollute global scope. load / store / call_indirect can be mapped to low level wasm instruction directly so we keep them in global scope. But for alwaysInline and unchecked, I think we should place them in a special namespace or start with some special prefix.
For C++, std library used compiler-implemented function will start with __. some builtin function will start with __builtin. I wonder should we also do something like this?
e.g.
use this like
builtin_opt.unchecked(....)
builtin_opt.alwaysInline(....);
|
Sounds like a good idea, but I'm hesitant to rename/move |
At least we can start from |
How about to use |
|
I'll do that. What do you think about a namespace for |
CountBleck
force-pushed
the
inline-builtin
branch
from
db37514 to
165cfea
Compare
There was a problem hiding this comment.
I still have some concern about adding not so important function in global scope. It may break some code and it is quiet annoyed to find a new suitable function name for user.
Some search result from github, since AS does not have special language identifier, I search for all function named inlined in TS.
https://github.com/search?q=%22+inlined%28%22+language%3ATypeScript+&type=code
I also checked SourceGraph and can't get anything relevant. I'm not opposed to the namespace idea, but idk if it'd inconvenience users without much of a benefit. |
|
Could this perhaps be a function associated with the existing |
I'd tend to agree with this approach, and it can be done like so: /** Annotates a method, function or constant global as always inlined. */
declare function inline(...args: any[]): any;
declare namespace inline {
/** Annodates a function call as inlined */
function always(...args: any[]): any;
} |
|
I didn't realize I left this hanging for so long, sorry! |
This builtin operates similar to unchecked, by setting a new FlowFlag that is checked in makeCallDirect in the area where the @inline decorator is checked, thereby achieving the same functionality as it.
CountBleck
force-pushed
the
inline-builtin
branch
from
165cfea to
76968d9
Compare
5 participants
Changes proposed in this pull request:
⯈ Add an
alwaysInlinebuiltin that inlines any inner function calls